Menus and React Testing Library migration #359

nelsonni · 2021-03-06T21:50:04Z

Description:

The initial interaction points within the Synectic canvas have been based on buttons managed within their respective card, stack, error, and dialog components. This is not a sustainable architecture going forward. A unified navigation menu, similar to the design considerations of https://material.io/components/menus, has prompted a refactoring to a single NavMenu component that launches all interactions via Redux actions.

Additionally, we have been maintaining tests written for both Enzyme and React Testing Library (RTL). However, issues such as #357 highlight that this structure is untenable as dependencies for their testing libraries begin to conflict. Its time to cut-over to using React Testing Library (RTL) only, and any remaining Enzyme tests need to be updated.

This PR resolves #90, #357, and #113, and signifies the following version changes:

MAJOR version increase
MINOR version increase
PATCH version increase

Changes:

This PR makes the following changes:

Moves @testing-library/user-event from dependencies to devDependencies in package.json to match other @testing-library/* packages.
@testing-library/dom added to devDependencies to allow DOM queries to work in conjunction with React Testing Library.
Bump react-dnd-test-utils from 11.1.3 to 12.0.0 to take advantage of the updated wrapWithTestBackend that wraps components with a DnDContext and returns [Component, getBackend] utilities; this removed the need for dndReduxMock.wrapInReduxContext in our test mocks.
Card.saveable field type dropped in favor of Metafile.state field type.
Editor cards able to save content when Metafile.state indicates a modified state.
FilePickerDialog converted to a function returning a Redux Thunk for handling calls to Electron.dialog.showOpenDialog.
filePickerDialog displays an Electron.dialog.showOpenDialog based on a new pickerType: openFile | openDirectory parameter for accepting only files or directories (defaults to both only on MacOS platforms, see Electron.remote.dialog.showOpenDialog limits FilePicker dialog to directory selector on Windows and Linux #113).
Button components removed from NewCardDialog, DiffPickerDialog, RepoBranchList, FilePickerDialog, MergeDialog.
All dialogs added to canvas through Redux actions (via Modal types) instead of state flags that control component rendering.
GitGraph added to canvas background through Redux actions (via the new Modal types).
Setting AceEditor basePath and modePath for dynamic importing of syntax highlighting modes in Jest testing environments (see Jest is unable to resolve file-loader! imports for react-ace/webpack-resolver.js #90).
__test__/__fixtures__/* created to hold a single consistent Redux store (along with valid Canvas, Card, Stack, Modal, and Metafile entries) for reduced boilerplate code in tests.
Enzyme deprecated and removed from testing environment (switched Jest testEnvironment: 'enzyme' to testEnvironment: 'jest-environment-jsdom-sixteen' based on recommendations documented in TypeError: MutationObserver is not a constructor #357).
React Testing Library (RTL) will remain the only testing framework, in conjunction with the Jest environment, for testing React components and container functions.
Simplified the custom useDirectory React Hook to no longer rely on changes occurring in metafile.ts for determining the status of child directories and files within a root directory; instead the only dependencies are in io.ts and state is determined by checking with the underlying filesystem. This fixes underlying memory leaks and Windows OS issues (see useDirectory mock must return fetch with () => Promise<void> type #321).

Checklist:

Before submitting this PR, I have verified that my code:

Resides on a fix/ or feature/ branch that was initially branched off from development.
Passes code linting (yarn lint) and unit testing (yarn test).
Successfully builds a distribution package (yarn package).

Additionally, I have verified that:

My name is listed in the Contributors section, or this PR includes a request to add my name.
I have read and am aware of the CONTRIBUTING guide for this project.

…on.dialog.showOpenDialog

… section

… during testing; resolves #90

…Dependency)

…mory leaks

jettseale

When I try to open a directory (File -> Open Directory...) it results in an error in the console and the file explorer card doesn't appear.

nelsonni · 2021-03-09T01:38:59Z

Analyzing this issue shows two likely separate problems on the Windows platform, that originate in the git.getStatus function:

git.getRepoRoot returns . instead of undefined on empty directories. This function relies on calling isomorphic-git/findRoot to determine the correct path containing a .git directory or file, but the issue appears to be related to directories not under version control.

synectic/src/containers/git.ts

Lines 290 to 293 in 07a420f

    
           export const getStatus = async (filepath: fs.PathLike): Promise<GitStatus | undefined> => { 
        
             const repoRoot = await getRepoRoot(filepath); 
        
             if (!repoRoot) return undefined; // no root Git directory indicates that the filepath is not part of a Git repo

hidefile/shouldBeHiddenSync throws the Uncaught (in promise) TypeError: Cannot read property 'getAttributesSync' of undefined error when a directory path is passed to it.

synectic/src/containers/git.ts

Lines 301 to 309 in 07a420f

    
             /** isomorphic-git provides `status()` for individual files, but requires `statusMatrix()` for directories  
        
              * (per: https://github.com/isomorphic-git/isomorphic-git/issues/13) */ 
        
             if (await io.isDirectory(filepath)) { 
        
               const statuses = await isogit.statusMatrix({ fs: fs, dir: dir, gitdir: gitdir, filter: f => !shouldBeHiddenSync(f) }); 
        
               const changed = statuses 
        
                 .filter(row => row[1] !== row[2])   // filter for files that have been changed since the last commit 
        
                 .map(row => row[0]);                // return the filenames only 
        
               return (changed.length > 0) ? 'modified' : 'unmodified'; 
        
             }

nelsonni · 2021-03-09T03:54:04Z

There is an open issue, stevenvachon/hidefile#3, that suggests others are also having trouble with the hidefile package on Windows. Added comment stevenvachon/hidefile#3 (comment) to that issue.

Fix in branch fix/windows-hidefiles should temporarily alleviate the issue by switching hidefile calls to use the underlying winattr package instead when the platform is Windows. @jettseale Please try this fix branch and see if it allows you to open directories again.

nelsonni · 2021-03-11T01:32:08Z

After testing the fix/windows-hidefiles branch with @jettseale, we determined that there are overlapping issues:

hidefiles is failing to run on Windows platforms. So calls to hidefile/shouldBeHiddenSync and winattr/getSync both fail (hidefile depends on winattr, so the likely culprit is in winattr).
The switch from useCallback to useEffect in the useDirectory custom React Hook (via b440fe6) is actually introducing a memory leak whenever a Explorer card is stacked and unstacked. The observed effect is that the contents of the Explorer card must reload, and when they do there is a chance that the following error will be thrown:

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.

nelsonni · 2021-03-11T02:23:00Z

The leaking memory error has actually been occurring on Windows for awhile now. After going back through the project history, we were able to narrow it down to being introduced sometime between c88e8cb (August 25, 2020) and e743c32 (September 11, 2020) on the development branch.

Also, it appears that the hidefile package was added during this development window as well (in commit 98817b1 on September 2, 2020).

…ndows OS

nelsonni added 28 commits March 4, 2021 15:39

Migrate @testing-library/user-event from dependencies to devDependencies

11b4751

Card.saveable migrated to Metafile.state

718cebf

Initial Editor-only saving logic

0c1e2eb

Modal subsumes Error for types, actions, and reducers

15fee9a

Card backs stylized grey instead of white to match theme

c47595d

Electron ApplicationMenu configuration added

b3f7dfc

ModalComponent added to capture dialogs and errors

dfaca05

Consolidate JSDoc status table for git.getStatus

9e02e2c

CanvasMenu upgraded to use NavMenu menu bar instead of buttons

371c714

FilePickerDialog converted to redux-thunk function for calling Electr…

dbb3513

…on.dialog.showOpenDialog

NewCardDialog button logic subsumed by dialog component

8c2a77f

DiffPickerDialog button logic subsumed by dialog component

7ed903b

RepoBranchList button logic subsumed by status component

721a0a5

Tests updated to use modals instead of errors

d016ac9

MergeDialog button logic subsumed by dialog component

6af4a8d

Cards default to below menu bar

65ae53c

Add missing @testing-library/dom dependency for React Testing Library…

076d22e

… section

Partial refactoring of DiffPickerDialog tests to RTL

9341dad

Deprecate Enzyme and switch to Testing Library with Jest only

de2e2a8

DiffPickerDialog testing migrated to RTL

1edbfe2

Set AceEditor 'basePath' and 'modePath' for dynamic importing in Jest…

c0a9ea2

… during testing; resolves #90

Bump react-dnd-test-utils from 11.1.3 to 12.0.0 (add @types/node peer…

868334d

…Dependency)

ContainsRequiredMetafile renamed to MetafileWithContains for clarity

c0b86b3

Typo fixed in Metafile.filetype JSDoc comments

32b02dc

Test fixtures created to reduce test suite boilerplate code

eef1bb0

Deprecated Enzyme testing and replaced with React Testing Library

a9edfd7

useDirectory.fetch migrated to useEffect and cleanup fn to prevent me…

b440fe6

…mory leaks

Remove working test file for migrating to RTL

85c8b39

nelsonni added bug Bug reports or bug fixes feature Feature requests or improvements labels Mar 6, 2021

nelsonni added this to the v1.0.0 milestone Mar 6, 2021

nelsonni requested a review from jettseale March 6, 2021 21:50

nelsonni added 5 commits March 6, 2021 14:35

Remove Enzyme dependencies

7cd23b2

Update Browser and Explorer tests for dndReduxMock deprecation

4af3a5c

Updated documentation to reflect Enzyme removal and RTL promotion

e6d1661

Remove unused and incomplete CanvasMenu components

e14d072

GitGraph repo selection integrated into NavMenu

07a420f

jettseale suggested changes Mar 9, 2021

View reviewed changes

Merge branch 'development' into feat/saving-changes

440e794

This was linked to issues Mar 9, 2021

Upgrading to React@17 #358

Closed

TypeError: MutationObserver is not a constructor #357

Closed

Electron.remote.dialog.showOpenDialog limits FilePicker dialog to directory selector on Windows and Linux #113

Closed

nelsonni added 4 commits March 12, 2021 11:01

Simplified useDirectory hook and async tests setup

8d3288d

isHidden shim function added to use winattr instead of hidefile on Wi…

a07cd1e

…ndows OS

removing winattr dependency

cc9f610

isHidden fn on Windows only examines for paths beginning with '.'

0171d76

jettseale self-requested a review March 12, 2021 23:01

jettseale previously approved these changes Mar 12, 2021

View reviewed changes

nelsonni dismissed jettseale’s stale review via 0171d76 March 12, 2021 23:05

jettseale self-requested a review March 13, 2021 00:10

jettseale approved these changes Mar 13, 2021

View reviewed changes

nelsonni merged commit 7685fee into development Mar 13, 2021

nelsonni deleted the feat/saving-changes branch March 13, 2021 00:16

nelsonni linked an issue Mar 13, 2021 that may be closed by this pull request

Jest is unable to resolve file-loader! imports for react-ace/webpack-resolver.js #90

Closed

nelsonni mentioned this pull request Mar 16, 2021

Unable to check file attributes for hidden flag on Windows OS #379

Closed

nelsonni mentioned this pull request Jul 1, 2021

Upgrade to React 17 #453

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Menus and React Testing Library migration #359

Menus and React Testing Library migration #359

nelsonni commented Mar 6, 2021 •

edited

Loading

jettseale left a comment

nelsonni commented Mar 9, 2021 •

edited

Loading

nelsonni commented Mar 9, 2021 •

edited

Loading

nelsonni commented Mar 11, 2021 •

edited

Loading

nelsonni commented Mar 11, 2021 •

edited

Loading

Menus and React Testing Library migration #359

Menus and React Testing Library migration #359

Conversation

nelsonni commented Mar 6, 2021 • edited Loading

Description:

Changes:

Checklist:

jettseale left a comment

Choose a reason for hiding this comment

nelsonni commented Mar 9, 2021 • edited Loading

nelsonni commented Mar 9, 2021 • edited Loading

nelsonni commented Mar 11, 2021 • edited Loading

nelsonni commented Mar 11, 2021 • edited Loading

nelsonni commented Mar 6, 2021 •

edited

Loading

nelsonni commented Mar 9, 2021 •

edited

Loading

nelsonni commented Mar 9, 2021 •

edited

Loading

nelsonni commented Mar 11, 2021 •

edited

Loading

nelsonni commented Mar 11, 2021 •

edited

Loading